Add aws-lc-rs crypto backend with trait-based abstraction#320
Add aws-lc-rs crypto backend with trait-based abstraction#320preuss-adam wants to merge 4 commits into
Conversation
- Add traits.rs with Backend, KeyPairImpl, PrivateKeyImpl, PublicKeyImpl - Add KeySerialization and KeyPemSerialization traits - Wire mod traits into crypto module
- Move ed25519.rs and p256.rs into default/ submodule - Implement KeyPairImpl, PrivateKeyImpl, PublicKeyImpl traits on default backend types - Rewrite mod.rs to use DefaultBackend + trait-based type aliases - Update scope.rs Display impl for new PublicKey type
- Implement Ed25519 and secp256r1 PublicKey, PrivateKey, KeyPair - Add AwsLcPublicKey trait with blanket KeyPemSerialization impl - Add P256 point compression/decompression via aws-lc-sys FFI - Add aws-lc-rs, aws-lc-sys deps; upgrade pkcs8 to 0.10.2 - Remove unused deps: sha2, prost-types, getrandom, ecdsa, elliptic-curve - Add tests for sign/verify, serialization, PEM/DER roundtrips
- Make ed25519-dalek, p256, aws-lc-rs, aws-lc-sys optional deps - Add default-backend and awslc-backend feature flags - Add default-backend-pem synthetic feature for dalek pem/pkcs8 - Make pem feature standalone - Gate awslc module with not(default-backend) to prevent conflicting impls - default-backend takes priority when both features are enabled
|
hi, as an update, i still plan on reviewing this; thanks for your patience. |
divarvel
left a comment
divarvel
left a comment
There was a problem hiding this comment.
One of the pending projects that’s related to this effort is allowing the use of a KMS, same as what was done in biscuit-java eclipse-biscuit/biscuit-java#113
I think the traits, as they are designed, would work, but the naming would be a bit misleading, as they focus on abstracting over things (public key, private key) instead of the actual behavior (signing, verifying).
The traits as they are defined have a granularity that would fit the bill (especially separating the key serialization functions), but the traits constraints as they stand would not allow using a KMS for the authority key and a regular keypair for attenuation keys. I feel that would be resolved by adding an equivalent of BiscuitBuilder::build that would be generic on the signer argument (maybe with a default defined with cfg macros).
Finally, and that’s really nitpicking, I was a bit taken aback by the xxImpl naming for a trait.
I think we should design things around two main traits:
Signer, used in BiscuitBuilder::build() and ThirdPartyRequest::create_block(). types implementing it are able to sign a payload, expose a public key and declare their algorithm.
KeyPair, used in Biscuit::append(). Types implementing it can be generated, expose a public and private key that can be serialized / deserialized and declare their algorithm.
Those two traits would handle the places where the biscuit impl itself needs to call out to the crypto layer. For the rest, there is no strong need to be generic, since it’s code written by library users (still, defining a trait would make things a bit nicer to user by enforcing consistency between implementations).
imo, moving parametricity to actual call sites in the biscuit library would let us get on by with smaller traits and simpler code, and default generic types defined through cfg attributes would keep the nice property of deciding which implementation to use through cargo features, while keeping the opportunity to strategically override things
2 participants
Summary
default/backend implementing the new traitsdefault-backend(ed25519-dalek/p256) andawslc-backend(aws-lc-rs)Benchmarks
Ed25519 verification (third-party checks)
P256 verification (third-party checks)
Usage
Default (ed25519-dalek + p256):
aws-lc-rs backend: